Skip to content

[FIX] dvb: bound entry_id in CLUT parsing to prevent heap buffer overflow - #2441

Open
Aditya-Saroha wants to merge 2 commits into
CCExtractor:masterfrom
Aditya-Saroha:fix/dvb-clut-entry-id-oob
Open

[FIX] dvb: bound entry_id in CLUT parsing to prevent heap buffer overflow#2441
Aditya-Saroha wants to merge 2 commits into
CCExtractor:masterfrom
Aditya-Saroha:fix/dvb-clut-entry-id-oob

Conversation

@Aditya-Saroha

@Aditya-Saroha Aditya-Saroha commented Nov 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

In raising this pull request, I confirm the following (please check boxes):

Reason for this PR:

  • This PR adds new functionality.
  • This PR fixes a bug that I have personally experienced or that a real user has reported and for which a sample exists.
  • This PR is porting code from C to Rust.

Sanity check:

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • If the PR adds new functionality, I've added it to the changelog. If it's just a bug fix, I have NOT added it to the changelog.
  • I am NOT adding new C code unless it's to fix an existing, reproducible bug.

Repro instructions:

Reproduction

Generate a minimal Transport Stream with out-of-bounds CLUT entries using the generator script below:

python3 gen_clut_poc.py clut_poc.ts
ccextractor clut_poc.ts --out=spupng

Master (compiled with -fsanitize=undefined):

src/lib_ccx/dvb_subtitle_decoder.c:1218:16: runtime error: index 4 out of bounds for type 'uint32_t [4]'
src/lib_ccx/dvb_subtitle_decoder.c:1219:16: runtime error: index 4 out of bounds for type 'uint8_t [4]'
src/lib_ccx/dvb_subtitle_decoder.c:1223:17: runtime error: index 255 out of bounds for type 'uint32_t [16]'
src/lib_ccx/dvb_subtitle_decoder.c:1224:17: runtime error: index 255 out of bounds for type 'uint8_t [16]'

Branch (compiled with -fsanitize=undefined):

DVB subtitle PID 33 language: eng
dvbsub_parse_clut_segment(): More than one bit level marked: e0
100%  |  00:00
Done, processing time = 0 seconds

Runs cleanly with zero UBSan errors.

Note on AddressSanitizer (ASan): ASan reports nothing on master because clut4[4], clut16[16], and clut256[256] are contiguous members within the heap-allocated struct DVBSubCLUT (1396 bytes). Out-of-bounds writes stay entirely within this allocation block, so ASan redzones are not triggered. The out-of-bounds indexing constitutes undefined behaviour that silently corrupts neighbouring palette entries, which UBSan detects immediately.

Summary & Root Cause

Fixes #2440.

In src/lib_ccx/dvb_subtitle_decoder.c:dvbsub_parse_clut_segment(), entry_id (parsed as an 8-bit uint8_t from the stream) was used unchecked as an array index into clut->clut4[entry_id] (size 4) and clut->clut16[entry_id] (size 16).

According to ETSI EN 300 743 §7.2.4 (CLUT definition segment):

  • 2-bit CLUT entries have entry_id in range 0..3 (clut4[4], ilut4[4]).
  • 4-bit CLUT entries have entry_id in range 0..15 (clut16[16], ilut16[16]).
  • 8-bit CLUT entries have entry_id in range 0..255 (clut256[256], ilut256[256]).

When processing an out-of-range entry (e.g. entry_id = 4 with 2-bit flag, or entry_id = 255 with 4-bit flag), writes overflow the array bounds:

  • clut4[4] lands +16 bytes into clut16[0].
  • clut16[255] lands +1020 bytes into clut256.

This undefined behaviour corrupts neighbouring palette entries in struct DVBSubCLUT.

Fix Details

  1. In-Condition Bounds & Fallthrough:
    Bound checks are placed directly inside each depth conditional branch: 
    if ((depth & 0x80) && entry_id < 4)
{
    clut->clut4[entry_id] = RGBA(r, g, b, 255 - alpha);
    clut->ilut4[entry_id] = y;
}
else if ((depth & 0x40) && entry_id < 16)
{
    clut->clut16[entry_id] = RGBA(r, g, b, 255 - alpha);
    clut->ilut16[entry_id] = y;
}
else if (depth & 0x20)
{
    clut->clut256[entry_id] = RGBA(r, g, b, 255 - alpha);
    clut->ilut256[entry_id] = y;
}
    Invalid entries are skipped without aborting or dropping the rest of the CLUT definition segment.
  2. Multi-Depth Flags Support:
    Per ETSI EN 300 743 §7.2.4, an entry targeting multiple depths (e.g. flags = 0xE0 with entry_id = 10) falls through the 2-bit check and correctly updates the 4-bit and 8-bit palettes.
  3. Stream Pointer Alignment:
    Color fields are read and buf is advanced (4 bytes for compact, 6 bytes for full range) ahead of the depth checks, ensuring skipped entries still consume their bytes and preserve pointer alignment for subsequent entries.
  4. Code Quality:
    • Removed the dead entry_id >= 256 check (1-byte uint8_t).
    • Added standard braces { } to if (buf_size < 2) { return -1; } and all conditional branches.

Verification

  • UBSan: Verified clean run on branch with zero runtime errors.
  • Pointer Advance: Verified with debug prints that subsequent entries (Entry 3 and Entry 4) are applied correctly after skipped entries.
  • Real DVB Samples Regression: Verified master and branch outputs are 100% byte-identical on real streams (dvbteletext.ts, sample_121.ts) across both --out=srt (diff -u returns 0) and --out=spupng (diff -r returns 0 on all generated PNG subpicture bitmaps).

TS Generator Script (gen_clut_poc.py)

Click to expand standalone Python generator 
#!/usr/bin/env python3
"""
gen_clut_poc.py -- Generates a minimal DVB subtitle Transport Stream
triggering out-of-bounds entry_id indexing in dvbsub_parse_clut_segment().
"""

import struct
import sys

def crc32_mpeg(data: bytes) -> int:
    crc = 0xFFFFFFFF
    for byte in data:
        crc ^= byte << 24
        for _ in range(8):
            if crc & 0x80000000:
                crc = ((crc << 1) ^ 0x04C11DB7) & 0xFFFFFFFF
            else:
                crc = (crc << 1) & 0xFFFFFFFF
    return crc

def ts_pkt(pid: int, payload: bytes, pusi: bool = False, cc: int = 0) -> bytes:
    flags1 = ((1 if pusi else 0) << 6) | ((pid >> 8) & 0x1F)
    flags2 = pid & 0xFF
    ctrl   = 0x10 | (cc & 0x0F)
    hdr = bytes([0x47, flags1, flags2, ctrl])
    payload_space = 188 - 4
    if len(payload) < payload_space:
        payload = payload + bytes([0xFF] * (payload_space - len(payload)))
    return hdr + payload[:payload_space]

def null_pkt() -> bytes:
    return ts_pkt(0x1FFF, b"")

def build_pat_section() -> bytes:
    body = struct.pack(">HH", 0x0001, 0xE020)
    section_length = 5 + len(body) + 4
    raw = bytes([
        0x00,
        0xB0 | (section_length >> 8), section_length & 0xFF,
        0x00, 0x01,
        0xC1,
        0x00, 0x00,
    ]) + body
    return raw + struct.pack(">I", crc32_mpeg(raw))

SUBTITLE_PID = 0x0021

def build_pmt_section() -> bytes:
    desc = bytes([
        0x59, 0x08,
        0x65, 0x6E, 0x67,  # 'eng'
        0x10,
        0x00, 0x01,
        0x00, 0x01,
    ])
    es_entry = struct.pack(">BHH", 0x06, 0xE000 | SUBTITLE_PID, 0xF000 | len(desc)) + desc
    pmt_body = struct.pack(">HH", 0xE1FF, 0xF000) + es_entry
    section_length = 4 + len(pmt_body) + 4
    raw = bytes([
        0x02,
        0xB0 | (section_length >> 8), section_length & 0xFF,
        0x00, 0x01,
        0xC1,
        0x00, 0x00,
    ]) + pmt_body
    return raw + struct.pack(">I", crc32_mpeg(raw))

def dvb_seg(seg_type: int, page_id: int, payload: bytes) -> bytes:
    return struct.pack(">BBHH", 0x0F, seg_type, page_id, len(payload)) + payload

def build_page_comp() -> bytes:
    body = bytes([0xFF, 0x08]) + struct.pack(">BBHH", 0x01, 0x00, 0x0000, 0x0000)
    return dvb_seg(0x10, 0x0001, body)

def build_region_comp() -> bytes:
    body = bytes([0x01, 0x00]) + struct.pack(">HH", 320, 50) + bytes([
        (2 << 5) | (2 << 2) | 0x00,
        0x01,
        0x00, 0x00,
        0x00, 0x00,
    ])
    return dvb_seg(0x11, 0x0001, body)

def clut_compact(entry_id: int, depth_flags: int, y: int, cr: int, cb: int, alpha: int) -> bytes:
    flags = (depth_flags & 0xE0) | 0x00
    b0 = (y & 0xFC) | ((cr >> 6) & 0x03)
    b1 = ((cr << 2) & 0xC0) | ((cb >> 2) & 0x30) | ((alpha >> 4) & 0x0C)
    return bytes([entry_id, flags, b0, b1])

def build_clut() -> bytes:
    hdr = bytes([0x01, 0x00])
    entries = b""
    entries += clut_compact(4,   0x80, 100, 50, 60, 200)  # Entry 1: 2-bit OOB (4 >= 4)
    entries += clut_compact(255, 0x40, 120, 80, 40, 64)   # Entry 2: 4-bit OOB (255 >= 16)
    entries += clut_compact(10,  0xE0, 150, 100, 70, 90)  # Entry 3: 2-bit OOB, 4-bit valid
    entries += clut_compact(1,   0x40, 145, 54, 34, 0)    # Entry 4: valid 4-bit entry
    return dvb_seg(0x12, 0x0001, hdr + entries)

def build_end_of_set() -> bytes:
    return dvb_seg(0x80, 0x0001, b"")

def build_sub_pes() -> bytes:
    segments = build_page_comp() + build_region_comp() + build_clut() + build_end_of_set()
    sub_payload = bytes([0x20, 0x00]) + segments + bytes([0xFF])
    pes_len = len(sub_payload) + 3
    if pes_len > 65535:
        pes_len = 0
    hdr = bytes([0x00, 0x00, 0x01, 0xBD]) + struct.pack(">H", pes_len) + bytes([0x80, 0x00, 0x00])
    return hdr + sub_payload

def main():
    out_path = sys.argv[1] if len(sys.argv) > 1 else "clut_poc.ts"
    pkts = []
    cc = {}
    def next_cc(pid: int) -> int:
        cc[pid] = (cc.get(pid, -1) + 1) & 0x0F
        return cc[pid]

    pat = bytes([0x00]) + build_pat_section()
    pkts.append(ts_pkt(0x0000, pat, pusi=True, cc=next_cc(0x0000)))

    pmt = bytes([0x00]) + build_pmt_section()
    pkts.append(ts_pkt(0x0020, pmt, pusi=True, cc=next_cc(0x0020)))

    pes = build_sub_pes()
    offset = 0
    pusi = True
    while offset < len(pes):
        chunk = pes[offset:offset + 184]
        pkts.append(ts_pkt(SUBTITLE_PID, chunk, pusi=pusi, cc=next_cc(SUBTITLE_PID)))
        offset += len(chunk)
        pusi = False

    while len(pkts) < 16:
        pkts.append(null_pkt())

    ts = b"".join(pkts)
    with open(out_path, "wb") as f:
        f.write(ts)
    print(f"Generated {len(ts)} bytes ({len(pkts)} packets) -> {out_path}")

if __name__ == "__main__":
    main()

@Aditya-Saroha
[FIX] dvb: bound entry_id in CLUT parsing to prevent heap buffer over…
acfced8 
…flow

According to ETSI EN 300 743 Section 7.2.4, 2-bit CLUT entries have
entry_id in range 0..3, 4-bit entries in range 0..15, and 8-bit entries in
range 0..255.

Previously, dvbsub_parse_clut_segment() directly used entry_id (up to 255)
as an index into clut->clut4[entry_id] (4 uint32_t entries) and
clut->clut16[entry_id] (16 uint32_t entries) without validating the bounds
against the palette bit depth. An untrusted or malformed stream could write
beyond the boundaries of clut4/clut16, corrupting adjacent heap memory
(including the clut->next pointer).

This commit:
- Enforces strict upper bounds on entry_id for 2-bit (entry_id < 4),
  4-bit (entry_id < 16), and 8-bit (entry_id < 256) CLUT entries.
- Adds bounds check for CLUT segment payload size (buf_size >= 2).
- Validates entry length against remaining buffer before reading compact
  or full_range color definitions to prevent out-of-bounds heap reads.
@cfsmp3 cfsmp3 closed this Nov 10, 2026
@cfsmp3 cfsmp3 reopened this Nov 10, 2026
cfsmp3
cfsmp3 requested changes Nov 10, 2026
View reviewed changes

@cfsmp3 cfsmp3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is Carlos' claude doing the first pass. Before Carlos takes a look, please address these issues.

The out-of-range index is real. A TS with a CLUT segment containing entry_id=4 with the 2-bit flag (or entry_id=255 with the 4-bit flag) gives UBSan index 4 out of bounds for type 'uint32_t [4]' at dvb_subtitle_decoder.c:1218 (and index 255 … [16] at :1223) on master. The 5 real DVB samples we have produce identical output on this branch.

  1. Don't drop the whole segment for one entry. In EN 300 743 the three entry-CLUT flags are independent: one entry may target several depths (e.g. flags 0xE0, entry_id 10 is valid for the 4-bit and 8-bit CLUTs). With return -1 in the depth & 0x80 branch, such an entry now discards the rest of the CLUT definition. Please put the bound in each condition and fall through, the way FFmpeg's dvbsubdec.c does, and skip only the invalid entry: 
    if ((depth & 0x80) && entry_id < 4)
{
	...
}
else if ((depth & 0x40) && entry_id < 16)
{
	...
}
else if (depth & 0x20)
{
	...
}
  2. Remove the entry_id >= 256 check. entry_id is read from one byte, so it can never be true.
  3. Braces: if (buf_size < 2) return -1; needs { }.
  4. Correct the description. The indices can't reach clut->next or heap metadata: the largest write (clut16[255] / ilut16[255]) still lands inside struct DVBSubCLUT, in the clut256 / ilut256 arrays. ASan reports nothing on master for this input. The effect is undefined behaviour that corrupts neighbouring palette entries. Please also attach a TS sample (or a generator) instead of the in-function snippet.

@Aditya-Saroha
dvb: skip out-of-range CLUT entries without dropping segment
e63ca21 
Per review on PR CCExtractor#2441:
- Bound entry_id within condition branches to skip invalid entries instead of dropping the segment
- Delete redundant entry_id >= 256 check (entry_id is 8-bit)
- Add braces to buf_size < 2 check
@Aditya-Saroha

Aditya-Saroha commented Nov 10, 2026

Copy link
Copy Markdown
Contributor Author

Hi Carlos,

Thank you for the review. All points have been addressed in the latest commit, and the PR description has been updated:

  1. Skip invalid entries without dropping segment: Moved bounds checks into the condition branches ((depth & 0x80) && entry_id < 4, (depth & 0x40) && entry_id < 16), allowing independent depth flags (e.g. 0xE0) to fall through properly while consuming payload bytes for skipped entries.
  2. Removed entry_id >= 256 check: Deleted the redundant check since entry_id is an 8-bit uint8_t.
  3. Braces: Added standard { } braces to if (buf_size < 2) { return -1; }.
  4. Corrected description & reproduction: Updated the PR description with the corrected root cause (UB corrupting neighbouring palette entries within struct DVBSubCLUT, which UBSan catches while ASan does not) and attached the standalone Python TS generator (gen_clut_poc.py).

Verified that UBSan runs completely clean on the test stream and that output on real DVB samples (dvbteletext.ts, sample_121.ts) remains 100% byte-identical to master.

@ccextractor-bot

ccextractor-bot commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator
CCExtractor CI platform finished running the test files on linux. 172/238 tests matched the approved output:
Report Name Tests Passed
Broken 9/13
CEA-708 2/14
DVB 0/7
DVD 3/3
DVR-MS 2/2
General 26/28
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 69/86
Teletext 0/21
WTV 12/13
XDS 32/34

66 tests do not match the approved output. That is the pass/fail verdict. Whether this branch caused it is a separate question, answered below.

Compared with the tip of master — test 9974, commit cccad8f:

  • 0 pass there and fail here
  • 0 fail there and pass here
  • 0 fail on both, with different output
  • 66 fail on both, byte for byte the same

Compared with the commit this branch was cut from — test 9878, commit 185fe3f:

  • 0 pass there and fail here
  • 0 fail there and pass here
  • 0 fail on both, with different output
  • 66 fail on both, byte for byte the same

No test changes behaviour relative to the tip of master: every failure above fails there too, byte for byte. The approved output for those tests is out of date, which is a baseline to review rather than a regression in this branch.

@ccextractor-bot

ccextractor-bot commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator
CCExtractor CI platform finished running the test files on windows. 172/238 tests matched the approved output:
Report Name Tests Passed
Broken 9/13
CEA-708 2/14
DVB 0/7
DVD 3/3
DVR-MS 2/2
General 26/28
Hardsubx 1/1
Hauppage 3/3
MP4 3/3
NoCC 10/10
Options 69/86
Teletext 0/21
WTV 12/13
XDS 32/34

66 tests do not match the approved output. That is the pass/fail verdict. Whether this branch caused it is a separate question, answered below.

Compared with the tip of master — test 9979, commit f4519ff:

  • 0 pass there and fail here
  • 0 fail there and pass here
  • 0 fail on both, with different output
  • 66 fail on both, byte for byte the same

Compared with the commit this branch was cut from — test 9693, commit af4b28d:

  • 0 pass there and fail here
  • 157 fail there and pass here
  • 42 fail on both, with different output
  • 24 fail on both, byte for byte the same
  • 1 have no result in that run

Fail on both but produce different output — behaviour moved even though the verdict did not:

Fail there, pass here:


No test changes behaviour relative to the tip of master: every failure above fails there too, byte for byte. The approved output for those tests is out of date, which is a baseline to review rather than a regression in this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cfsmp3 cfsmp3 cfsmp3 requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] DVB subtitle decoder: heap-buffer-overflow write in dvbsub_parse_clut_segment via unbounded entry_id

3 participants

@Aditya-Saroha @ccextractor-bot @cfsmp3